Skip to content

Fix uninitialized CUB reduction identity corrupting fill_holes vertices - #41

Open
rwfsmith wants to merge 1 commit into
JeffreyXiang:mainfrom
rwfsmith:fix/fill-holes-uninitialized-identity
Open

Fix uninitialized CUB reduction identity corrupting fill_holes vertices#41
rwfsmith wants to merge 1 commit into
JeffreyXiang:mainfrom
rwfsmith:fix/fill-holes-uninitialized-identity

Conversation

@rwfsmith

Copy link
Copy Markdown

Summary

CuMesh::fill_holes produces corrupted vertices whenever the CUB reduction identity happens to land on non-zero host stack memory. In practice the first fill_holes call in a process is usually fine and every subsequent call is corrupted, which makes this look like non-determinism but is actually deterministic per-call-index state.

This is a cross-platform correctness bug, not a Windows-specific one — the identity is constructed on the host, so it depends on host stack residue, not on the compiler or OS.

Root cause

fill_holes computes each hole-cap vertex with a segmented reduction over Vec3f:

// src/clean_up.cu
cub::DeviceSegmentedReduce::Sum(..., (Vec3f*)sums, num_holes, offsets, offsets + 1, stream);
inplace_div_kernel<<<...>>>(sums, counts, num_holes);

CUB materializes the reduction identity on the host as InitT{}. Vec3f is declared with a user-provided default constructor:

// src/dtypes.cuh
struct __align__(16) Vec3f {
    float x, y, z;
    __device__ __forceinline__ Vec3f();   // user-provided => Vec3f{} is NOT zero-initialized
    ...
};

Two things combine:

  1. Because the default constructor is user-provided, Vec3f{} is value-initialization that calls the constructor rather than zero-filling.
  2. The constructor is marked __device__ only, so it is not callable from host code. CUB's host-side InitT{} therefore yields whatever bytes were on the host stack.

That garbage G is then folded into every segment's sum and divided by the segment size, so each bad vertex gets (G + Σ midpoints) / n.

Evidence

Dumping the corrupted vertex buffer showed all bad Z values were exactly G / n for small integers n, with identical mantissas at different exponents:

0x4F3D360B = 3.17443550e+09   (G/3)
0x4EBD360B = 1.58721775e+09   (G/6)
0x4E3D360B = 7.93608875e+08   (G/12)
...
G ≈ 9,523,306,496

A single constant divided by the segment size can only come from a non-zero reduction identity. Only the Z component was affected here — the stack bytes backing x/y happened to be zero in this process, which is exactly the "sometimes it works" signature.

The bad vertices were also the contiguous tail of the buffer (the newly appended hole-cap vertices), and face indices were all in range — consistent with fill_holes appending bad vertices rather than any earlier stage emitting them.

Fix

Make Vec3f's constructors __host__ __device__ so the host-side identity is actually constructed (and zeroed) instead of reading uninitialized stack memory. Six lines, no behavior change on the device side.

atlas.cu already uses the safe pattern for its reduction (float3 with an explicit make_float3(0, 0, 0) identity); this brings fill_holes in line with that.

QEM in the same header has the same __device__-only user-provided constructor pattern, but it is not used in any CUB reduction today, so it is intentionally left unchanged to keep this fix surgical.

Validation

Repro harness: N image-to-3D generations in a single process (TRELLIS.2), measuring max mesh edge length, long-edge count, and rendered mask coverage.

run before after
0 ok — max edge 0.005, 0 long edges, 30.0% coverage ok
1 broken — max edge 4.76e9, 3910 long edges, 52.3% coverage ok
2 broken — identical corruption ok
3 broken — identical corruption ok
4 broken — identical corruption ok

0/5 clean before, 5/5 clean after. Renders confirm it visually: the corrupted runs smeared the model into a cylindrical column of sliver triangles; after the fix every run reconstructs correctly.

Built and tested on Windows 11, MSVC 19.44, CUDA 13.0, PyTorch 2.13, RTX 5090 (sm_120).

`CuMesh::fill_holes` computes each new hole-cap vertex with
`cub::DeviceSegmentedReduce::Sum` over `Vec3f` (src/clean_up.cu:660).
CUB builds the reduction's initial value **on the host** as `InitT{}`.

Because `Vec3f` declares a *user-provided* default constructor, `Vec3f{}`
performs value-initialization by calling that constructor rather than
zero-initializing the members. The constructor was marked `__device__`
only, so it is not callable from host code and the initial value was
left as raw host stack memory.

CUB adds that identity into every segment, so each hole-cap vertex became

    v_i = (garbage + sum(loop midpoints)) / n_i

i.e. every new vertex was offset by the same garbage vector scaled by
1/n_i. Whenever the stack slot happened to hold a large float, the newly
appended vertices were flung far outside the mesh, producing long sliver
triangles that render as heavy streaking. Components whose stack bytes
happened to be zero were unaffected, so the corruption typically showed
up in only one axis.

This is state-dependent rather than random: the first `fill_holes` call
in a process often lands on a zeroed stack slot and produces a correct
mesh, while every subsequent call reuses stack residue and reproduces the
same corruption. Observed on a TRELLIS.2 image-to-3D run, where the first
generation in a process was always clean and every later generation was
corrupted identically (26,556 bad vertices, max |z| ~4.8e9).

Marking the `Vec3f` constructors `__host__ __device__` makes the existing
zero-initializing constructor callable from host code, so CUB's identity
is a true zero. `atlas.cu` already uses the safe pattern for the analogous
`float3` reduction by passing `make_float3(0, 0, 0)` explicitly.

Verified on Windows 11 / CUDA 13.0 / RTX 5090 (sm_120) with 5 consecutive
image-to-3D generations in a single process: before the fix runs 1-7 were
all corrupted (max edge 4.76e9, 3910 degenerate long edges, silhouette
coverage 52.3% vs 30.0% expected); after the fix all runs are clean
(max edge ~0.005, 0 long edges, coverage 30.0%).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant